Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Extra test scenarios #208

Draft
wants to merge 6 commits into
base: 8.2.x
Choose a base branch
from
Draft

Conversation

gmponos
Copy link
Contributor

@gmponos gmponos commented Jun 20, 2020

I tried to improve the test case scenarios that you have.

I found a couple of bugs also.. Will report them upstream..

In the meantime you might be interested in some discussion about a couple of those and how to resolve them.

*
* @phpstan-param string $foo
* @psalm-param string $foo
* @phpstan-return true
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you want to handle these?

@@ -28,3 +29,5 @@
extract($bar);

compact('foo', 'bar');

array_map('is_null', ['1', '2', null]); // forbidden function will not be detected
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

FYI codesniffer can not detect these.. is this fine?

class FooError extends Error
{
}

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is superfluous but not reported.. i guess there is no existing rule for this

return true;
}

// Return true here in case $bar is bar
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

for (
$i = 0; $i < 10;
$i++
);
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@@ -23,6 +23,7 @@ class Example implements \IteratorAggregate
{
private const VERSION = \PHP_VERSION - (PHP_MINOR_VERSION * 100) - PHP_PATCH_VERSION;


Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

FYI currently there is no sniff included that fixes the extra lines inside a class.. it does fix the extra lines between functions but it does not look inside the whole class.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

SlevomatCodingStandard.Classes.ClassMemberSpacing

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great.. will create a new PR with that..

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There are also:

  • MethodSpacing
  • PropertySpacing
  • ConstantSpacing

@greg0ire greg0ire changed the base branch from master to 8.2.x October 25, 2020 10:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants